perf: every page's script becomes a cacheable file, not inline markup - #58
Merged
Conversation
Codacy carries a 60% coverage goal that has always read "not reported", because nothing ever produced a report. Measured: 51% overall. A separate, deliberately non-blocking job. It re-runs the four suites under tracing, which is slower, and the gate job should stay the fast answer — so this one only measures and never decides pass/fail. The summary lands in the job summary and coverage.xml/coverage.txt upload as artifacts. Where the misses are, for anyone reading the number: app.py 43% and ssh_manager.py 55% are mostly install/bootstrap and remote-management paths that need a real host to exercise, which is why they are low and why chasing the percentage for its own sake would be the wrong instinct. manage.py is 0% — the break-glass CLI a locked-out admin uses, and a genuine gap. auth.py 87% and models.py 90% are where this session's tests landed. Codacy upload needs a CODACY_PROJECT_TOKEN this repo does not have, so the step is present but commented out with the instructions rather than half-wired into a job that would fail on the missing secret. Verified by running the job's exact commands locally: 4 suites traced, combined, valid Cobertura XML out. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The base.html extraction covered the SHARED bundle; each page still inlined
its own, re-sent on every visit and cacheable by nobody. 306KB across seven
templates, now seven content-hashed static files behind the same week-long
Cache-Control.
remote_manage 86.6K server_detail 48.0K
server_files 45.3K manage_remotes 32.0K
dashboard 26.3K manage_servers 16.7K
remote_firewall 9.0K
First load, gzipped over the wire, at 5 servers:
/ 19.3 -> 11.0 KB
/server/1 25.4 -> 11.0 KB
/server/1/files 24.1 -> 11.5 KB
/remotes 17.7 -> 10.5 KB
and on any later visit the script is not sent at all.
Three pages were pure lift-and-shift: dashboard, manage_servers and
manage_remotes had no template syntax in their scripts whatsoever. The other
four hold between one and seven server-rendered values, all of them top-level
`var X = …;` — verified at brace depth 0 before splitting, because a var
inside an IIFE would have stopped being a global the moment it moved. Those
lines stay inline ahead of the file, exactly as base.html's do.
remote_manage's trailing UPro.load(...) call stays inline AFTER it, since it
calls into the code the file defines.
The guard added last change earned itself immediately: my extractor wrote the
island comments INSIDE the script elements, and "no Jinja tags in an inline
<script>" failed the moment I ran it — the same mistake, caught this time
before it reached CodeQL.
Two smoke checks grepped the rendered detail page for JS that is now a file.
They use the existing page_with_assets() helper, so they follow the reference
instead of asserting less; the markup-only check next to them still looks at
the HTML alone, because that is what it is about.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| ErrorProne | 10 high |
| Security | 90 critical |
🟢 Metrics 2432 complexity · 43 duplication
Metric Results Complexity 2432 Duplication 43
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The base.html change covered the shared bundle. Each page still inlined its own script — re-sent on every visit, cacheable by nobody. 306 KB across seven templates, now seven content-hashed static files behind the same week-long
Cache-Control.remote_manage.jsserver_detail.jsserver_files.jsmanage_remotes.jsdashboard.jsmanage_servers.jsremote_firewall.jsFirst load, gzipped over the wire, at 5 servers:
//server/1/server/1/files/remotes…and on any later visit the script isn't sent at all. Combined with the earlier change,
/has gone 38.7 → 11.0 KB.How the split was made safe
Three pages were pure lift-and-shift —
dashboard,manage_serversandmanage_remoteshad no template syntax at all in their scripts.The other four hold between one and seven server-rendered values, all of them top-level
var X = …;. I verified every one sits at brace depth 0 before splitting: avarinside an IIFE stops being a global the moment it moves to another file, and that would have broken the page silently. Those lines stay inline ahead of the file, exactly as base.html's do.remote_manage's trailingUPro.load(...)stays inline after it, since it calls into what the file defines.The guard paid for itself
My extractor wrote the island comments inside the script elements — the identical mistake that produced the two CodeQL
js/syntax-erroralerts last time. The check added in that fix failed the moment I ran it, so this one never left my machine.Tests
Two smoke checks grepped the rendered detail page for JS that's now a file. They use the existing
page_with_assets()helper so they follow the reference rather than asserting less; the markup-only check beside them still looks at the HTML alone, because that's what it's about.unit 839 · smoke 294 · rbac 65 · template_actions 5 · lint clean.
🤖 Generated with Claude Code